Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(app, app-shell, app-shell-odd): update electron version to 31.3.1 #15894

Open
wants to merge 9 commits into
base: edge
Choose a base branch
from

Conversation

shlokamin
Copy link
Member

@shlokamin shlokamin commented Aug 5, 2024

Overview

Building on top of @koji's work to update node, this PR updates electron to 31.3.1

Changelog

  • Update electron, electron-builder, and electron/rebuild

Review requests

  • Play around with the desktop app and make sure it works fine
  • Upload a system zip to a flex and make sure the ODD app works fine

Mac build: https://builds.opentrons.com/app/Opentrons-v7.5.0-mac-b46438-app_update-electron-app-build.dmg
Windows build: https://builds.opentrons.com/app/Opentrons-v7.5.0-win-b46438-app_update-electron-app-build.exe
Linux build: https://builds.opentrons.com/app/Opentrons-v7.5.0-linux-b46438-app_update-electron-app-build.AppImage

System Image (For Updates): https://builds.opentrons.com/ot3-oe/10354327595/ot3-system.zip
Full Image (For Flashing): https://builds.opentrons.com/ot3-oe/10354327595/ot3-fullimage.tar
Version file: https://builds.opentrons.com/ot3-oe/10354327595/VERSION.json

Risk assessment

High, this affects low level app and ODD infrastructure

@koji
Copy link
Contributor

koji commented Aug 5, 2024

probably we'd like to update
"@electron/rebuild"
"electron-builder"

probably optional but nice to have
"electron-store"
"electron-updater"

@@ -75,6 +75,7 @@
"@types/node-fetch": "2.6.11",
"@types/styled-components": "^5.1.26",
"axios": "^0.21.1",
"electron-updater": "6.2.1",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added electron-updater to the app's dev deps so we can use its types

@shlokamin
Copy link
Member Author

@koji i updated electron/rebuild, electron-builder, and electron-updater. im gonna leave electron-store alone for now since i don't want to mess with any of the app/ODD config infrastructure. we can do this in a separate PR

@shlokamin shlokamin marked this pull request as ready for review August 12, 2024 14:56
@shlokamin shlokamin requested review from a team as code owners August 12, 2024 14:56
@shlokamin shlokamin requested review from smb2268 and koji and removed request for a team August 12, 2024 14:56
@koji
Copy link
Contributor

koji commented Aug 15, 2024

@koji i updated electron/rebuild, electron-builder, and electron-updater. im gonna leave electron-store alone for now since i don't want to mess with any of the app/ODD config infrastructure. we can do this in a separate PR

that totally makes sense to me.

@@ -27,7 +27,7 @@
},
"packageManager": "[email protected]",
"engines": {
"node": "^18.19.0"
"node": ">=18.19.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry I haven't updated this since I just tested Nodejs's compatibility.
https://www.electronjs.org/docs/latest/tutorial/electron-timelines

Suggested change
"node": ">=18.19.0"
"node": ">=20.14.0"

@koji
Copy link
Contributor

koji commented Aug 16, 2024

For Desktop, I did smoke test. Most parts worked as expected but Redux Dev Tools didn't work anymore. I couldn't see any state information that I could see Electron v27. v27 sometimes shows the same screen but after re-opening the dev tools, I can see states in Redux Dev Tools 🤔

Screenshot 2024-08-16 at 12 57 16 PM
https://github.com/reduxjs/redux-devtools-extension/blob/master/docs/Troubleshooting.md#excessive-use-of-memory-and-cpu

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants